Skip to content

Fix race condition in multi_threaded_observable_map test#1550

Open
DefaultRyan wants to merge 2 commits intomasterfrom
1549-bug-test-multi_threaded_observable_map-fails-randomly
Open

Fix race condition in multi_threaded_observable_map test#1550
DefaultRyan wants to merge 2 commits intomasterfrom
1549-bug-test-multi_threaded_observable_map-fails-randomly

Conversation

@DefaultRyan
Copy link
Member

There are two issues causing failures in this test, with both of them resulting in the same problem: the background operation does its thing, but never fires the hook, which causes the foreground wait to timeout, releaseing the foreground. Under sufficient load, this can cause the foreground operation to run unsynchronized with the background, causing failures and crashes.

The actual container code still appears to be fine - this is a test-only issue.

First, concurrency_checked_random_access_iterator only overrode operator*, but not operator->. However, this caused GetMany() to not fire a hook because iterator::current_value_withlock() retrieves values differently for map values.

T current_value_withlock() const
{
WINRT_ASSERT(m_current != m_end);
if constexpr (!impl::is_key_value_pair<T>::value)
{
return m_owner->unwrap_value(*m_current);
}
else
{
return make<impl::key_value_pair<T>>(m_owner->unwrap_value(m_current->first), m_owner->unwrap_value(m_current->second));
}
}

Under load, the foreground wait would timeout, allowing it to call it.GetMany() before the background had initialized it, causing a crash.

Fixed by hooking operator-> in the concurrency test iterator.

Second, the MoveNext test cases aren't triggering the hook because iterator advancement isn't an operation with a hook. The test was authored to have the at hook trigger the foreground thread, but MoveNext() doesn't trigger this hook.

Under load, the foreground Remove/Insert can complete before the background call to MoveNext(), causing MoveNext() to move to the end of the container, causing the test check to fail.

@DefaultRyan DefaultRyan linked an issue Mar 19, 2026 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Test multi_threaded_observable_map fails randomly

3 participants